planner: prove null-reject for deferred expressions by winoros · Pull Request #67789 · pingcap/tidb

winoros · 2026-04-15T14:02:56Z

What problem does this PR solve?

Issue Number: close #67788, close #65583

Problem Summary:

Null-reject proof skipped Constant expressions with DeferredExpr to avoid treating plan-cache-sensitive runtime values as static constants. That was conservative, but it also missed safe symbolic proof opportunities and left a now-obsolete plan-cache compatibility parameter on planner/util.IsNullRejected.

What changed and how does it work?

This PR lets null-reject proof inspect DeferredExpr symbolically without evaluating or nullify-folding its runtime value.

Add an internal allowNullifiedFold mode to proveNullRejected. Normal proof still uses the nullify-then-fold bridge. When descending into Constant.DeferredExpr, the proof keeps using structural null-reject rules but disables nullified folding.
Keep ParamMarker and DeferredExpr out of static constant folding candidates.
Remove the obsolete skipPlanCacheCheck argument from IsNullRejected and update its callers.
Add regression cases for symbolic deferred proof, placeholder NULL classification, and ensuring deferred expressions skip nullified folding.

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No need to test
- I checked and no code files have been changed.

Unit tests:

go test -run TestIsNullRejectedProofModes -tags=intest,deadlock
./tools/check/failpoint-go-test.sh pkg/planner/core/operator/logicalop -run '^$'
make lint

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

Summary by CodeRabbit

Refactor
- Unified null-rejection logic in the query planner, affecting when joins can be simplified to inner joins and when NOT NULL predicates are derived or propagated.
Tests
- Expanded test coverage for deferred/runtime-dependent expressions and null-rejection proofing to validate the updated behavior.

ti-chi-bot · 2026-04-15T14:03:00Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

coderabbitai · 2026-04-15T14:03:04Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: cc2e6ac5-e95a-497d-8b8b-0e420f219d8b

📥 Commits

Reviewing files that changed from the base of the PR and between e301757 and d592612.

📒 Files selected for processing (6)

pkg/planner/core/operator/logicalop/logical_aggregation.go
pkg/planner/core/operator/logicalop/logical_join.go
pkg/planner/core/operator/logicalop/logical_projection.go
pkg/planner/util/funcdep_misc.go
pkg/planner/util/null_misc.go
pkg/planner/util/null_misc_test.go

🚧 Files skipped from review as they are similar to previous changes (3)

pkg/planner/util/funcdep_misc.go
pkg/planner/core/operator/logicalop/logical_projection.go
pkg/planner/core/operator/logicalop/logical_aggregation.go

📝 Walkthrough

Walkthrough

Removes the skipPlanCacheCheck parameter from IsNullRejected, adds an allowNullifiedFold proof flag, threads it through recursive null-rejection proof logic (suppressing nullify-then-fold for plan-cache-sensitive/deferred constants), and updates planner call sites and tests to the simplified IsNullRejected(...) signature.

Changes

Cohort / File(s)	Summary
Core null-rejection logic `pkg/planner/util/null_misc.go`	Remove `skipPlanCacheCheck` from `IsNullRejected` signature; add `allowNullifiedFold` flag; thread flag through `proveNullRejected` and helpers; prevent nullify-then-fold for deferred/plan-cache-sensitive constants.
Operator call sites (FD & join logic) `pkg/planner/core/operator/logicalop/logical_aggregation.go`, `pkg/planner/core/operator/logicalop/logical_projection.go`, `pkg/planner/core/operator/logicalop/logical_join.go`	Update all calls to `util.IsNullRejected(...)` to the new signature (drop trailing boolean), affecting NOT NULL derivation used in FD extraction and outer→inner join simplification and derived predicates.
Function dependency helper `pkg/planner/util/funcdep_misc.go`	Call site updated to call `IsNullRejected` without the extra boolean in `ExtractNotNullFromConds`.
Tests `pkg/planner/util/null_misc_test.go`	Add `newNullRejectDeferredConst` test helper; add deferred-constant test cases; update test calls to new `IsNullRejected` signature.

Sequence Diagram(s)

sequenceDiagram
    participant Op as Planner Operator (Proj/Agg/Join)
    participant IsNull as util.IsNullRejected
    participant Proof as proveNullRejected
    participant Fold as tryFoldNullifiedConstant

    Op->>IsNull: IsNullRejected(ctx, schema, expr)
    IsNull->>Proof: start proof (allowNullifiedFold = true)
    alt encounter Constant with DeferredExpr
        Proof->>Proof: descend into DeferredExpr (allowNullifiedFold = false)
        Proof->>Fold: do not attempt nullify-then-fold (blocked)
    else other expressions
        Proof->>Fold: may attempt nullify-then-fold
    end
    Proof-->>IsNull: result (null-rejected true/false)
    IsNull-->>Op: boolean result

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

planner: replace outer-join null-reject evaluation with structural proof | tidb-test=pr/2724 #67129: Modifies planner null-rejection logic and the IsNullRejected API/implementation in overlapping areas.

Suggested labels

ok-to-test

Suggested reviewers

guo-shaoge
Reminiscent
qw4990
AilinKid

Poem

🐰 I nibble through proofs with careful hop,
Deferred crumbs I sniff but never chop,
I peek inside their wrapped-up core,
Say “null’s rejected” — then I hop some more,
A tiny trace, the planner leaps a crop.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title 'planner: prove null-reject for deferred expressions' clearly and concisely describes the main change: enabling null-reject proof for deferred expressions.
Description check	✅ Passed	The PR description includes issue reference (`#67788`), problem summary, detailed explanation of changes, test coverage, and release notes section, matching the template structure.
Linked Issues check	✅ Passed	The PR fully addresses issue `#67788` objectives: allows symbolic null-reject proof for DeferredExpr, prevents runtime value folding, removes obsolete skipPlanCacheCheck parameter, and adds unit tests.
Out of Scope Changes check	✅ Passed	All code changes are scoped to the null-reject proof enhancement: updating null_misc.go signature, propagating callers in aggregation/join/projection/funcdep files, and adding regression tests.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

tiprow · 2026-04-15T14:03:05Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

pantheon-ai · 2026-04-15T14:04:33Z

Review Complete

Findings: 0 issues
Posted: 0
Duplicates/Skipped: 0

_{ℹ️ Learn more details on Pantheon AI.}

codecov · 2026-04-15T14:21:02Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.5719%. Comparing base (757952a) to head (d592612).
⚠️ Report is 11 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #67789        +/-   ##
================================================
- Coverage   77.6156%   77.5719%   -0.0437%     
================================================
  Files          1982       1966        -16     
  Lines        548909     551248      +2339     
================================================
+ Hits         426039     427614      +1575     
- Misses       122060     123632      +1572     
+ Partials        810          2       -808

Flag	Coverage Δ
integration	`41.0339% <90.0000%> (+6.6941%)`	⬆️
unit	`76.7927% <100.0000%> (+0.4518%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
dumpling	`61.5065% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`50.4780% <ø> (-9.9508%)`	⬇️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

winoros · 2026-04-15T17:32:07Z

/retest

ti-chi-bot · 2026-04-17T11:28:31Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: qw4990, Reminiscent

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~pkg/planner/OWNERS~~ [qw4990]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ti-chi-bot · 2026-04-17T11:28:38Z

[LGTM Timeline notifier]

Timeline:

2026-04-16 10:40:47.154695073 +0000 UTC m=+1644052.360055130: ☑️ agreed by Reminiscent.
2026-04-17 11:28:35.869594363 +0000 UTC m=+1733321.074954420: ☑️ agreed by qw4990.

winoros · 2026-04-17T12:14:26Z

/retest

winoros · 2026-04-17T13:11:45Z

/retest

pantheon-ai

✅ Code looks good. No issues found.

winoros · 2026-06-01T14:07:35Z

/cherry-pick release-8.5

ti-chi-bot · 2026-06-01T14:08:24Z

@winoros: new pull request created to branch release-8.5: #68850.
But this PR has conflicts, please resolve them!

Details

In response to this:

/cherry-pick release-8.5

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot Bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesn't merit a release note. labels Apr 15, 2026

ti-chi-bot Bot added the sig/planner SIG: Planner label Apr 15, 2026

ti-chi-bot Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 15, 2026

winoros marked this pull request as ready for review April 15, 2026 14:04

ti-chi-bot Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 15, 2026

winoros force-pushed the null-reject-opt branch from 0badcb2 to e301757 Compare April 15, 2026 14:23

planner: prove null-reject for deferred expressions

d592612

winoros force-pushed the null-reject-opt branch from e301757 to d592612 Compare April 15, 2026 14:52

Reminiscent approved these changes Apr 16, 2026

View reviewed changes

ti-chi-bot Bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Apr 16, 2026

qw4990 approved these changes Apr 17, 2026

View reviewed changes

ti-chi-bot Bot added approved lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Apr 17, 2026

winoros merged commit eea8b1e into pingcap:master Apr 17, 2026
35 checks passed

winoros deleted the null-reject-opt branch April 17, 2026 14:05

pantheon-ai Bot reviewed Apr 19, 2026

View reviewed changes

coderabbitai Bot mentioned this pull request Apr 25, 2026

planner: fix null-reject based FD not-null inference #68050

Merged

13 tasks

premal pushed a commit to premal/tidb that referenced this pull request Apr 30, 2026

planner: prove null-reject for deferred expressions (pingcap#67789)

6ba6c63

winoros mentioned this pull request May 14, 2026

planner: plan cache wrongly skipped when outer-join null-reject inference involves parameters #65583

Closed

ti-chi-bot mentioned this pull request Jun 1, 2026

planner: prove null-reject for deferred expressions (#67789) #68850

Open

13 tasks

winoros added a commit to ti-chi-bot/tidb that referenced this pull request Jun 1, 2026

This is an automated cherry-pick of pingcap#67789

d554da6

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

Conversation

winoros commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

Summary by CodeRabbit

Uh oh!

ti-chi-bot Bot commented Apr 15, 2026

Uh oh!

coderabbitai Bot commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Uh oh!

tiprow Bot commented Apr 15, 2026

Uh oh!

pantheon-ai Bot commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

winoros commented Apr 15, 2026

Uh oh!

ti-chi-bot Bot commented Apr 17, 2026

Uh oh!

ti-chi-bot Bot commented Apr 17, 2026

[LGTM Timeline notifier]

Uh oh!

winoros commented Apr 17, 2026

Uh oh!

winoros commented Apr 17, 2026

Uh oh!

Uh oh!

pantheon-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

winoros commented Jun 1, 2026

Uh oh!

ti-chi-bot commented Jun 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

winoros commented Apr 15, 2026 •

edited

Loading

coderabbitai Bot commented Apr 15, 2026 •

edited

Loading

pantheon-ai Bot commented Apr 15, 2026 •

edited

Loading

codecov Bot commented Apr 15, 2026 •

edited

Loading